fix(gorilla-merger): custom StoreAPI over tsdb.DB (bypass thanos TSDBStore crash, #39) - #317
Merged
Merged
Conversation
…Store crash, #39) thanos store.TSDBStore.Series fatally OOMs ("runtime: out of memory", ~8EB) when thanos-query issues a Series RPC against the merger. Under Prometheus's default `stringlabels` build labels.Labels is a packed struct{ data string } (16 bytes), not a []Label slice. TSDBStore.Series builds storepb labels via labelpb.ZLabelsFromPromLabels (an unsafe *(*[]ZLabel)(unsafe.Pointer(&lset)) reinterpret assuming the []Label layout) and then wraps the stream in a resortingServer whose Send calls ReAllocZLabelsStrings(..,false) -> string(noAllocBytes(name)). On the packed-string layout those reads see garbage string lengths and the process dies. thanos v0.41.0 is the latest release and requires Go 1.25, so there is no version/toolchain escape. Replace store.NewTSDBStore with a thin custom storepb.StoreServer (customStore) over the embedded *tsdb.DB: - Series: ChunkQuerier(sorted=true) -> for each series, append external labels once via ExtendSortedLabels (dedup, external wins), build ZLabels by COPYING each Name/Value (zLabelsCopy) instead of the unsafe ZLabelsFromPromLabels, and emit raw XOR AggrChunks (copied bytes). Sends directly, NO resortingServer/flushable wrapper (Querier sorting is sufficient), so ReAllocZLabelsStrings is never invoked. - LabelNames/LabelValues: query the range and merge external label names/values. - Info: unchanged behaviour (external label set + min/max time + TsdbInfos), now sourced from customStore via the safe copying path. Unit tests drive Series in-process with a fake Store_SeriesServer: round-trip (no crash; labels = metric+attrs+external once with no dups; chunks decode back to the ingested samples), SkipChunks, external-label gating, and LabelNames/LabelValues. Full suite + go vet pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The crash
The merger ingests fragments fine and ships 2h blocks to S3 fine, but when thanos-query issues a Series RPC the merger fatally crashes:
Root cause: under Prometheus's default
stringlabelsbuild,labels.Labelsis a packedstruct{ data string }(verified:sizeof(labels.Labels) == 16), not a[]Labelslice (24 bytes). thanosstore.TSDBStore.Series:labelpb.ZLabelsFromPromLabels, a zero-copy*(*[]ZLabel)(unsafe.Pointer(&lset))reinterpret that assumes the[]Labellayout — on the packed-string layout it reads the 16-byte struct as a 24-byte slice header, producingZLabels with garbage string lengths;resortingServerwhoseSendcallsReAllocZLabelsStrings(.., false)->string(noAllocBytes(name)), which then tries to materialize a ~8EB string from a corrupt length -> OOM.thanos v0.41.0 is the latest release and requires Go 1.25, so there is no version/toolchain fix. Two earlier fixes are already merged (Info-service registration #313, stop double-stamping external labels), but the crash is inside thanos's own
TSDBStore.Seriespath so it persisted.The fix — custom StoreServer
Replace
store.NewTSDBStorewith a thin customstorepb.StoreServer(customStore) over the embedded*tsdb.DB. It avoids both unsafe paths:ChunkQuerier(MinTime, MaxTime)->Select(ctx, sorted=true, hints, matchers...). For each series, append the merger's external labels once vialabelpb.ExtendSortedLabels(dedup, external wins; built with alabels.Builderso it's a normal allocation that outlives the querier), then build thestorepb.SeriesZLabels by copying eachName/Value(zLabelsCopy) — neverZLabelsFromPromLabels. Chunks are emitted as rawstorepb.AggrChunk{Raw: &Chunk{Type: XOR, Data: <copied bytes>}}perchunks.Meta(storepb encoding = tsdb encoding - 1). Each is sent directly viasrv.Send(storepb.NewSeriesResponse(...)). NoresortingServer/newFlushableServer/newBatchableServerwrapper —Querier(sorted=true)already yields series ordered by label set and the consistent external-label append preserves that order, soReAllocZLabelsStringsis never invoked.SkipChunkssends labels only.TsdbInfos), now sourced fromcustomStorevia the same safe copying path (ZLabelSetsFromPromLabels, which copies field-by-field).Tests
New
customstore_test.godrives theSeriesRPC in-process with a fakestorepb.Store_SeriesServerthat collects responses (no real gRPC connection needed):TestCustomStoreSeriesRoundTrip(the key proof): ingest two series through the real HTTP ingest handler, then callSeries. Asserts (1) it does not crash/OOM and returns no error, (2) returned labels == metric labels + attrs + external labels once, no duplicates, (3) series are sorted by label set, (4) the returned XOR chunks decode back to the exact ingested samples.TestCustomStoreSeriesSkipChunks:SkipChunks=truereturns labels (external appended once) and zero chunks.TestCustomStoreSeriesExternalLabelGate: a matcher on an external label with a non-matching value yields 0 series; a matching value yields 1 (matcher satisfied by the appended label, not passed to the querier).TestCustomStoreLabelNamesValues: LabelNames/LabelValues merge the external label and return the stored ones.go build ./...,go vet ./..., andgo test ./... -count=1all pass (built withGOPRIVATE=github.com/ProjectASAP/*for the privateasap-gorilla-gomodule).Test plan
cd gorilla-merger && GOPRIVATE='github.com/ProjectASAP/*' go build ./... && go vet ./... && go test ./... -count=1🤖 Generated with Claude Code